Skip to content

Conversation

@f-bor
Copy link

@f-bor f-bor commented Nov 1, 2025

adding SRs on multiple usb devices may fail because /usr/lib/udev/scsi_id returns the same device id for all the usb devices.

This PR fixes this by checking the drive type and using the device serial number if correctly read.

@f-bor f-bor force-pushed the fix/multiple-usb-devices branch from 6727526 to 9946f5e Compare November 1, 2025 18:32
@Wescoeur Wescoeur requested review from Nambrok and Wescoeur November 14, 2025 09:26
@Wescoeur
Copy link
Member

Hi! Thank you for your contribution. Could you please sign-off your commit after taking the suggestions into account?

@Wescoeur
Copy link
Member

Also IMHO a commit like Use serial instead of SCSI ID for USB devices is probably more explicit.

@f-bor f-bor changed the title fix: add multiple usb SRs Use serial instead of SCSI ID for USB devices Nov 15, 2025
@f-bor f-bor force-pushed the fix/multiple-usb-devices branch 2 times, most recently from e464d66 to 9802153 Compare November 15, 2025 21:39
drivers/util.py Outdated
Comment on lines 1073 to 1074
except CommandException:
usb_device_with_serial = False
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
except CommandException:
usb_device_with_serial = False
except Exception as e:
usb_device_with_serial = False
SMlog(f"Unable to check if device is USB: {e}")

Copy link
Author

@f-bor f-bor Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a big fan of broad exception catch but ok. Regarding the log, just using f-string interpolation on exception is often not very helpful. Would it be ok for you to do the log like this:

Suggested change
except CommandException:
usb_device_with_serial = False
except:
usb_device_with_serial = False
SMlog("Unable to check if device is USB:")
SMlog(traceback.format_exc())

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can commit your suggestion, not a blocking point I guess.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok it is committed.

@f-bor f-bor force-pushed the fix/multiple-usb-devices branch from 9802153 to ce6361c Compare November 25, 2025 06:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants